Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: vat warehouse for LRU demand paged vats #2784

Merged
merged 22 commits into from
Jun 9, 2021
Merged

feat: vat warehouse for LRU demand paged vats #2784

merged 22 commits into from
Jun 9, 2021

Conversation

dckc
Copy link
Member

@dckc dckc commented Apr 1, 2021

was designed to fix #2277, but some stuff below remains; perhaps for issues to-be-filed...
leaving aside restore from snapshot for the initial version

(no longer based on #2370)

@dckc dckc changed the base branch from master to 2273-snapstore April 1, 2021 15:39
@dckc dckc changed the title 2277 vatmgrmgr feat: vat warehouse for LRU demand paged vats (WIP) Apr 1, 2021
@dckc dckc force-pushed the 2277-vatmgrmgr branch 2 times, most recently from db12211 to 485be22 Compare April 1, 2021 16:50
@dckc dckc force-pushed the 2277-vatmgrmgr branch 2 times, most recently from fa6df42 to e6446f6 Compare April 1, 2021 19:45
Base automatically changed from 2273-snapstore to master April 1, 2021 21:45
@dckc dckc force-pushed the 2277-vatmgrmgr branch 2 times, most recently from 794cbf1 to 10c4d44 Compare May 18, 2021 00:39
@dckc dckc force-pushed the 2277-vatmgrmgr branch 8 times, most recently from 88d3016 to 2ec7821 Compare May 25, 2021 04:46
@dckc
Copy link
Member Author

dckc commented May 25, 2021

@warner adding that throw in panic() causes an unhandled rejection in test/test-exomessages.js. (Yarn reports that it passes, but exits with a non-0 code, causing ci failure. Better than sweeping it under the rug!) I managed to diagnose it, but I'd like your help fixing it. I've exhausted my budget for flailing about by myself.

ref 299d55c

@dckc dckc changed the title feat: vat warehouse for LRU demand paged vats (WIP) feat: vat warehouse for LRU demand paged vats May 25, 2021
@dckc dckc requested a review from warner May 25, 2021 21:08
dckc added 21 commits June 8, 2021 18:45
After thorough investigation, let's be explicit that panic() does
not (unconditionally) throw but rather normally returns.

Add explicit default value for err arg too.

Also, clean up a couple awaits on resolveToError(), which does not
return a Promise.
initial policy: most recently used 20 vats are kept online

Note that it's important to handle rejection in wasTerminated: in
addition to the error path for the failed execution, there's an error
generated when shutting down the xsnap subprocess.

 - buildKernel:
   - makeVatWarehouse takes over
     - instantiating static, dynamic vats in start()
     - ephemeral.vats
       - vatWarehouse.lookup() replaces ephemeral.vats.has()
     - addVatManager, removeVatManager
     - deliverToVat
   - re-wire notifyTermination
 - makeVatLoader:
   - create methods return Promise<VatManager>
   - translators are passed in
   - no longer uses vatNameToID, queueToExport
   - notifyTermination has been hoisted to its own file
   - clarify source bundle type
 - test vat-target: exhibit stateful behavior
 - replace boolean shortcuts with helper function
 - convert lookup from arrow function
 - note optimization opportunity: replay vats in parallel
 - note makeVatTranslators precondition
 - punt on provideVatKeeper refactor
 - punt on note about comms vat source repeated

We had a TODO to "add a way to remove a vatKeeper from ephemeral in
kernel.js so that we can get rid of a vatKeeper when we evict its
vat"; I tried a `pruneVatKeeper()` method on `kernelKeeper` but it
failed with `resolution of "kp40" is still pending`.
 - provide type for kernel
 - factor out defensiveCopy
Thanks for review, BW.
@dckc dckc enabled auto-merge (squash) June 8, 2021 23:48
@dckc
Copy link
Member Author

dckc commented Jun 8, 2021

Looks good: please make the one await change in ensureVatOnline, then rebase to trunk

done.

We need to make additional tickets to address the other issues I spotted

right; I down-graded "fixes 2277" until we have those tickets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

design vatWarehouse API for demand-paged vats
3 participants